Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

P1: CRUD usage examples moved to appropriate pages #618

Merged
merged 27 commits into from
Mar 24, 2025

Conversation

rachel-mack
Copy link
Contributor

@rachel-mack rachel-mack commented Feb 19, 2025

Pull Request Info

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-47267

Staging Links

  • crud/bulk
  • crud/delete
  • crud/insert
  • crud/query-documents/find
  • crud/update-documents
  • Self-Review Checklist

    • Is this free of any warnings or errors in the RST?
    • Did you run a spell-check?
    • Did you run a grammar-check?
    • Are all the links working?
    • Are the facets and meta keywords accurate?

    Sorry, something went wrong.

    @rachel-mack rachel-mack requested a review from a team as a code owner February 19, 2025 20:25
    @rachel-mack rachel-mack requested review from katcharov and removed request for a team February 19, 2025 20:25
    @rachel-mack rachel-mack marked this pull request as draft February 19, 2025 20:25
    Copy link

    netlify bot commented Feb 19, 2025

    Deploy Preview for docs-java ready!

    Name Link
    🔨 Latest commit 7028bdd
    🔍 Latest deploy log https://app.netlify.com/sites/docs-java/deploys/67e1aa2fdbcaa30008fa69bd
    😎 Deploy Preview https://deploy-preview-618--docs-java.netlify.app
    📱 Preview on mobile
    Toggle QR Code...

    QR Code

    Use your smartphone camera to open QR code link.

    To edit notification comments on pull requests, go to your Netlify site configuration.

    @rachel-mack rachel-mack changed the title Move BulkWrite example P1: CRUD usage examples moved to appropriate pages Feb 25, 2025
    @rachel-mack rachel-mack marked this pull request as ready for review February 25, 2025 16:46
    Copy link
    Contributor

    @norareidy norareidy left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Left some suggestions!

    Copy link
    Contributor

    @norareidy norareidy left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Left some more suggestions, they're all pretty minor but lmk if you want me to take another look

    @rachel-mack
    Copy link
    Contributor Author

    @katcharov This is ready for tech review.

    The .java files have been relocated and a few of them merged together. They have all been run locally.

    @rachel-mack
    Copy link
    Contributor Author

    rachel-mack commented Mar 4, 2025

    This ready for tech review.
    NOTE: Some of the documents have been moved, but the only content that has been changes is under the <Operator> Example: Full File heading on each page.
    All examples have been tested locally.

    Copy link
    Collaborator

    @katcharov katcharov left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Partial review (Retrieve section)

    Comment on lines 65 to 67
    To retrieve a single document, you can append the ``first()`` method to your
    ``find()`` operation. You can use the ``sort()`` operation before selecting the first
    document to help choose the correct file.
    Copy link
    Collaborator

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Should this discuss any other methods, apart from first()?

    To retrieve a single document, you can use the first() method with a find() method. To select a particular document based on ordering, use the sort() method prior to applying first().

    • "append" is odd.
    • Let's avoid the term "correct", except to identify actual correctness issues. This suggests first does not behave correctly unless sort is supplied, but it is often fine to select an arbitrary first item.
    • I think "file" is intended to be a synonym for "document" but this should be avoided since we do not deal with files, and we should refer to documents as such.
    • Somewhat technical, but the sort is part of the find operation. find() is a method.

    If we are counting on using first, and it is worth suggesting sort, it is probably worth suggesting limit due to this.

    There are also a few unusual or imprecise phrasings in the paragraphs above:

    Use the find operation to retrieve a subset of your existing data in MongoDB. You can specify what data to return including which documents to retrieve, in what order to retrieve them, and how many to retrieve.

    • "what data to return including which documents to retrieve" suggests that there might be something else besides documents
    • "subset" and "existing" seem unnecessary: "Use the find operation to retrieve data from MongoDB"?

    To perform a find operation, call the find() method on an instance of a MongoCollection.

    • Somewhat technical, but the operation is not performed (sent to the server and executed there) until a method like "first" (or iterator, etc.) is called.

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    I've made some changes here. The only thing I wasn't sure what to do about is your last point:

    To perform a find operation...

    If someone executes the following code:

    collection.find(filter)

    Does the server execute a find operation? I understand there a distinction between a method and an operation, but the method is the vehicle by which a user causes a find operation to be performed, correct? Like are we just talking about semantics, or is this something that the reader would find confusing about how this written?

    Copy link
    Collaborator

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    I appreciate you checking to make sure this is clear.

    collection.find(filter) ... Does the server execute a find operation?

    No, this actually happens only after a method like first (or: executeExplain, iterator, cursor, forEach, ...) is invoked. The preceding methods, like sort, build something like an BSON document representing the operation. Those final methods eventually send BSON to the server, where the operation is executed. Calling the find method on an instance of a collection will not initiate a search on the server. (Not that we need to document all of this, but the docs should not imply otherwise.)

    (I do realize that some of this goes beyond what was added in this PR, so it is fine if it is addressed in another ticket.)

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    OK, thanks for the clarification. I've made some changes.

    @rachel-mack rachel-mack requested a review from katcharov March 7, 2025 20:54
    @rachel-mack rachel-mack force-pushed the DOCSP-47267-bulkwrite branch from 827b3b3 to afb49e7 Compare March 12, 2025 01:41
    @rachel-mack rachel-mack requested a review from katcharov March 12, 2025 01:48
    Copy link
    Collaborator

    @katcharov katcharov left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Just a few minor things, otherwise LGTM!

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh

    Verified

    This commit was signed with the committer’s verified signature.
    rachel-mack Rachel Mackintosh
    @rachel-mack rachel-mack force-pushed the DOCSP-47267-bulkwrite branch from e71bc53 to 7028bdd Compare March 24, 2025 18:53
    @rachel-mack rachel-mack merged commit 0434019 into mongodb:master Mar 24, 2025
    5 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    None yet
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    None yet

    3 participants